[Android] Adding Hyphenation Frequency prop for Text component #29157

fabOnReact · 2020-06-17T15:53:24Z

Summary

This issue fixes #28279
android_hyphenationFrequency prop for Android Text component which sets the frequency of automatic hyphenation to use when determining word breaks.

Changelog

[Android] [Fixed] - Adding Hyphenation Frequency prop for Text component

Test Plan

More info are available in the android docs. I will add the documentation to the docs later once the pull request is taken in consideration for merging.

AFTER

I remain available to do improvements. Thanks a lot. Fabrizio.

analysis-bot · 2020-06-17T16:17:58Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	6,764,177	2,599
android	hermes	armeabi-v7a	6,426,979	2,183
android	hermes	x86	7,150,511	1,794
android	hermes	x86_64	7,040,444	1,748
android	jsc	arm64-v8a	8,937,601	2,633
android	jsc	armeabi-v7a	8,592,787	2,221
android	jsc	x86	8,767,045	1,815
android	jsc	x86_64	9,342,605	1,758

Base commit: e75557b

analysis-bot · 2020-06-17T16:20:02Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	829440	n/a

Base commit: e75557b

elicwhite · 2020-06-18T02:04:59Z

I can't confirm that this will get approved (I don't have context on the Android side), but we've been following a pattern of the platform prefixes being android_, so this prop should be android_hyphenationFrequency. This is also seen in these props on Pressable: https://github.com/facebook/react-native/blob/master/Libraries/Components/Pressable/Pressable.js#L122-L127

fabOnReact · 2020-06-18T07:28:37Z

thanks a lot @TheSavior . Changes are included in 4500095. I'm fixing the same issue also in #29059. I have no prerogative for the PR to be merged, but just hope to get a better insight in the react-native project and write more relevant pr in the future.. Thanks a lot. Fabrizio

JoshuaGross · 2020-06-24T22:09:34Z

This seems pretty reasonable to me :)

facebook-github-bot

@JoshuaGross has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

JoshuaGross · 2020-06-25T04:07:24Z

@fabriziobertoglio1987 I'm working on testing internally and having some conversations about the prefix. While I'm doing that, could you issue a PR to the docs site to add documentation for the new prop?

https://github.com/facebook/react-native-website/tree/master/docs

fabOnReact · 2020-06-26T06:12:56Z

UPDATE 30/06/2020

I notice now that 0fda91f does not include the latest changes. Tomorrow I will work on testing this functionality on Android API < 23 and add changes from 14709f8 to master if required (in a separate pull request). Additionally I noticed my errors in #29070. Sorry for this, I did a mistake. I will fix #29070 and not make any errors in my future pull request. Thanks a lot and sorry for disturbing. I understand now that ReactNative Core team is very busy and I should deliver higher quality code.

OLD MESSAGE 26/06/2020

Thanks a lot @JoshuaGross I added the docs and also added a new commit to this Pull Request 14709f8 as setHyphenationFrequency android method is available only from API 23+.

I decided to write this PR as a solution of #28279 to give users more control on the Text Component. I don't work on iOS and I had the impression that textBreakingStrategy was related to breaking paragraphs and sentences (justifying a different prop and setter).

I tested on RNTester example emulator API 28.

#28940 unluckily denies me to test this changes on my API 19 emulator. I am on cellular data right now and can not download another Android 22 image to test the code changes from 14709f8 on devices < Android 23, but I believe the logic will work as the same logic is used with textBreakingStrategy

react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java

Lines 551 to 555 in ce0edca

    
           @ReactProp(name = ViewProps.TEXT_BREAK_STRATEGY) 
        
           public void setTextBreakStrategy(@Nullable String textBreakStrategy) { 
        
             if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) { 
        
               return; 
        
             }

I'll work on this and let you know. I remain available. Thanks a lot. I wish you a good day. Fabrizio

react-native-bot · 2020-06-30T00:00:04Z

This pull request was successfully merged by @fabriziobertoglio1987 in 0fda91f.

^{When will my fix make it into a release? | Upcoming Releases}

JoshuaGross · 2020-07-01T18:08:00Z

@fabriziobertoglio1987 Thank you for following up on this! The amendment is being merged in.

Summary: JoshuaGross This issue fixes #28279 as discussed in #29157 (comment) Avoid calling [setHyphenationFrequency](https://developer.android.com/reference/android/widget/TextView#setHyphenationFrequency(int)) on Android Sdk < 23. ## Changelog  [Android] [Fixed] - do not call setHyphenationFrequency on AndroidSdk < 23 Pull Request resolved: #29258 Test Plan: | **BEFORE** | **AFTER** | |:-------------------------:|:-------------------------:| | <img src="https://user-images.githubusercontent.com/24992535/86214122-05bf0e00-bb7b-11ea-93b5-2174812bfec9.png" width="300" height="" />| <img src="https://user-images.githubusercontent.com/24992535/86214130-08216800-bb7b-11ea-9fc0-68b28638bf57.png" width="300" height="" /> | The warning displayed with `adb logcat | grep -P "ReactTextAnchorViewManager"` ![image](https://user-images.githubusercontent.com/24992535/86214242-34d57f80-bb7b-11ea-9945-30ae25332bfb.png) I remain available to do improvements. Thanks a lot. Fabrizio. Reviewed By: JoshuaGross Differential Revision: D22337095 Pulled By: mdvacca fbshipit-source-id: d0943397af180929c48044ccbc7a9388549021b8

fabOnReact · 2020-07-03T08:57:41Z

#29258

Following same implementation used with underlineColorAndroid https://github.com/fabriziobertoglio1987/react-native/blob/15810e96d90e18dbd424666338fdec0127d403ed/Libraries/Components/TextInput/TextInput.js#L470 https://github.com/fabriziobertoglio1987/react-native/blob/15810e96d90e18dbd424666338fdec0127d403ed/Libraries/Components/TextInput/AndroidTextInputNativeComponent.js#L203 While with other components (for example prop android_ripple), we use the android_ prefix to denote platform specific props. facebook#29157 (comment) https://github.com/fabriziobertoglio1987/react-native/blob/15810e96d90e18dbd424666338fdec0127d403ed/Libraries/Components/Pressable/Pressable.js#L177 In the case of TextInput we already have Platform Logic that detects Android/iOS platform. https://github.com/fabriziobertoglio1987/react-native/blob/15810e96d90e18dbd424666338fdec0127d403ed/Libraries/Components/TextInput/TextInput.js#L1268 For this reason TextInput component does not use android_ props and instead uses this naming convention underlineColorAndroid. To be noted that the prop underlineColorAndroid is passed to both iOS and Android version, while other props have platform specific logic for android and iOS. https://github.com/fabriziobertoglio1987/react-native/blob/15810e96d90e18dbd424666338fdec0127d403ed/Libraries/Components/TextInput/TextInput.js#L1334 https://github.com/fabriziobertoglio1987/react-native/blob/15810e96d90e18dbd424666338fdec0127d403ed/Libraries/Components/TextInput/TextInput.js#L1293 Example of a prop that have a specific value on Android and is different from iOS https://github.com/fabriziobertoglio1987/react-native/blob/15810e96d90e18dbd424666338fdec0127d403ed/Libraries/Components/TextInput/TextInput.js#L1271 I decided to follow the same solution used in underlineColorAndroid.

fabOnReact added 3 commits June 17, 2020 17:30

adding prop type to Text component

430df65

android HyphenationFrequency setter

da73c2f

adding text for HyphenationFrequency

3660768

fabOnReact requested a review from shergin as a code owner June 17, 2020 15:53

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 17, 2020

react-native-bot added Bug Platform: Android Android applications. labels Jun 17, 2020

removing un-used constant from RNTester

f8b1f70

fabOnReact added 2 commits June 17, 2020 18:22

fix linter

25df8e3

fix linting issues

0c86fb4

fabOnReact mentioned this pull request Jun 17, 2020

command yarn run lint does not run on the correct files #29159

Closed

rename prop to android_hyphenationFrequency

4500095

facebook-github-bot reviewed Jun 24, 2020

View reviewed changes

elicwhite mentioned this pull request Jun 26, 2020

Handling platform specific properties react-native-community/discussions-and-proposals#188

Open

fabOnReact mentioned this pull request Jun 26, 2020

adding docs for android_hyphenationStrategy facebook/react-native-website#2007

Merged

avoid calling method for API < 21

14709f8

facebook-github-bot closed this in 0fda91f Jun 29, 2020

react-native-bot added the Merged This PR has been merged. label Jun 30, 2020

fabOnReact mentioned this pull request Jul 1, 2020

[Android] do not call setHyphenationFrequency on AndroidSdk < 23 #29258

Closed

fabOnReact deleted the hyphenation-frequency branch July 1, 2020 07:23

evd1ser mentioned this pull request Jul 19, 2021

Android android_hyphenationFrequency in text does not work in react-native 0.64.2 #31878

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Android] Adding Hyphenation Frequency prop for Text component #29157

[Android] Adding Hyphenation Frequency prop for Text component #29157

fabOnReact commented Jun 17, 2020 •

edited

Loading

analysis-bot commented Jun 17, 2020 •

edited

Loading

analysis-bot commented Jun 17, 2020 •

edited

Loading

elicwhite commented Jun 18, 2020

fabOnReact commented Jun 18, 2020

JoshuaGross commented Jun 24, 2020

facebook-github-bot left a comment

JoshuaGross commented Jun 25, 2020

fabOnReact commented Jun 26, 2020 •

edited

Loading

react-native-bot commented Jun 30, 2020

JoshuaGross commented Jul 1, 2020

fabOnReact commented Jul 3, 2020

[Android] Adding Hyphenation Frequency prop for Text component #29157

[Android] Adding Hyphenation Frequency prop for Text component #29157

Conversation

fabOnReact commented Jun 17, 2020 • edited Loading

Summary

Changelog

Test Plan

analysis-bot commented Jun 17, 2020 • edited Loading

analysis-bot commented Jun 17, 2020 • edited Loading

elicwhite commented Jun 18, 2020

fabOnReact commented Jun 18, 2020

JoshuaGross commented Jun 24, 2020

facebook-github-bot left a comment

Choose a reason for hiding this comment

JoshuaGross commented Jun 25, 2020

fabOnReact commented Jun 26, 2020 • edited Loading

react-native-bot commented Jun 30, 2020

JoshuaGross commented Jul 1, 2020

fabOnReact commented Jul 3, 2020

fabOnReact commented Jun 17, 2020 •

edited

Loading

analysis-bot commented Jun 17, 2020 •

edited

Loading

analysis-bot commented Jun 17, 2020 •

edited

Loading

fabOnReact commented Jun 26, 2020 •

edited

Loading